Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix execute_command test #109

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

Fix execute_command test #109

wants to merge 3 commits into from

Conversation

maksimdrachov
Copy link
Member

This fixes the failing unit test tests/cmd/execute_command.py.

The issue arose due to this implemented in public_regulated_data_types:

OpenCyphal/public_regulated_data_types@ec27883

maksimdrachov and others added 2 commits March 13, 2024 16:01
@maksimdrachov @pavel-kirienko
Add example for call command (#98)
15949ea 
Question:

Is there a way to this:

```
yakut call 125 435:uavcan.node.ExecuteCommand '{command: COMMAND_RESTART, parameter: "firmware.bin"}'
```


Instead of this:

```
yakut call 125 435:uavcan.node.ExecuteCommand '{command: 65533, parameter: "firmware.bin"}'
```

[Relevant forum
question](https://forum.opencyphal.org/t/yakut-file-commands/1815/2)

---------

Co-authored-by: Pavel Kirienko <pavel.kirienko@gmail.com>
@maksimdrachov
Fix execute_command test
38aae49
@maksimdrachov maksimdrachov changed the title Fix test Fix execute_command test Jul 25, 2024
@maksimdrachov
Copy link
Member Author

Ok, the same errors show up on my local setup:

image

@maksimdrachov
Copy link
Member Author

@pavel-kirienko

Can you weight in on why this is happening?

I checked .venv3.12/lib/python3.12/site-packages/pycyphal/transport/__init__.py, it has the following line:

from ._transfer import TransferFrom as TransferFrom
image

@maksimdrachov
Copy link
Member Author

Highlighted part is where the error occurs:

image

@maksimdrachov maksimdrachov mentioned this pull request Jul 27, 2024
Open
@pavel-kirienko
Copy link
Member

The MyPy errors could be related to this recent change in PyCyphal: OpenCyphal/pycyphal#343

PyCyphal imports were considered missing by MyPy until we added the py.typed label; now it's trying to parse PyCyphal but failing for some reason. We could simply force it to shut up by adding to setup.cfg:

 [mypy-pycyphal.*]
 ignore_missing_imports = True
+ignore_errors = True

But this is not a good solution for obvious reasons. However, it is often the case with MyPy that no solution is a good one.

@maksimdrachov I've been staging minor changes on the dev branch to avoid publishing releases too often; consider changing the merge target to that branch instead.

@maksimdrachov maksimdrachov changed the base branch from main to dev August 31, 2024 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@maksimdrachov @pavel-kirienko